Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

convert AssetKey to StrictModel #20643

Closed
wants to merge 2 commits into from
Closed

convert AssetKey to StrictModel #20643

wants to merge 2 commits into from

Conversation

sryza
Copy link
Contributor

@sryza sryza commented Mar 21, 2024

Summary & Motivation

How I Tested These Changes

@sryza sryza mentioned this pull request Mar 21, 2024
branch-name: strict-model
branch-name: asset-key-strict-model
@sryza sryza force-pushed the asset-key-strict-model branch from 313f29b to ead649f Compare March 21, 2024 23:46
@alangenfeld
Copy link
Member

given the very large volume of these objects we end up with, I think it would be good to do some quick bench marking to prove to ourselves that moving from tuple to model isn't a meaningful difference in memory / cpu

Base automatically changed from strict-model to master March 25, 2024 16:10
sryza added a commit that referenced this pull request Mar 25, 2024
## Summary & Motivation

Introduces a `StrictModel` class, which is a subclass of
`pydantic.BaseModel` that's frozen and doesn't allow default constructor
arguments that aren't class members.

The idea is for this to eventually replace all our uses of `NamedTuple`,
`BaseModel`, and `@dataclass`.

This is a replacement for the unmerged
#20461.

Downstream PRs that demonstrate its usage:
- #20641
- #20643
- #20638

## How I Tested These Changes
PedramNavid pushed a commit that referenced this pull request Mar 28, 2024
## Summary & Motivation

Introduces a `StrictModel` class, which is a subclass of
`pydantic.BaseModel` that's frozen and doesn't allow default constructor
arguments that aren't class members.

The idea is for this to eventually replace all our uses of `NamedTuple`,
`BaseModel`, and `@dataclass`.

This is a replacement for the unmerged
#20461.

Downstream PRs that demonstrate its usage:
- #20641
- #20643
- #20638

## How I Tested These Changes
@sryza sryza closed this Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants